From 1f83a2e791a502ba237b8eeb57a6720518419389 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 3 Jul 2012 22:55:43 +0200 Subject: [PATCH] (bug 38152) jquery.tablesorter: Use .data() instead of .attr() * .data() is faster when looking up repeatedly * .data() will return the latest value, even if it has changed over time, whereas .attr() only gives the value as it was when the page was generated. Much like the difference between attr() and prop() when it comes to value, checked, disabled etc. * Moved buildCache() call outside `if ( firstTime )`. Tests show that this is fast enough to do on the fly. Left inline comment with proposal on how to make this more elaborate in the future would it become necessary. * jquery.tablesorter.test.js: - Add tests for bug 38152 - Clean up double quotes - Use more descriptive id (prefix with mw-bug-) Change-Id: Ie4f801c5b1f93617fd3fd173d2eaaf173a7604e9 --- RELEASE-NOTES-1.20 | 3 + resources/jquery/jquery.tablesorter.js | 25 +++- .../jquery/jquery.tablesorter.test.js | 124 ++++++++++++++---- 3 files changed, 117 insertions(+), 35 deletions(-) diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20 index 75d770463d..d70b042955 100644 --- a/RELEASE-NOTES-1.20 +++ b/RELEASE-NOTES-1.20 @@ -149,6 +149,9 @@ upgrade PHP if you have not done so prior to upgrading MediaWiki. * (bug 31895) mw.loader mode now correct when triggered from a $.fn.ready handler that is bound before mediawiki.js's handler (e.g. browser-userscripts like greasemonkey). +* (bug 38152) jquery.tablesorter: Use .data() instead of .attr(), so that live + values are used instead of just the fixed values from when the tablesorter + was initialized. === API changes in 1.20 === * (bug 34316) Add ability to retrieve maximum upload size from MediaWiki API. diff --git a/resources/jquery/jquery.tablesorter.js b/resources/jquery/jquery.tablesorter.js index 21e1f968e8..08272a59e6 100644 --- a/resources/jquery/jquery.tablesorter.js +++ b/resources/jquery/jquery.tablesorter.js @@ -60,7 +60,7 @@ /* Local scope */ - var ts, + var ts, parsers = []; /* Parser utility functions */ @@ -77,9 +77,15 @@ function getElementText( node ) { var $node = $( node ), - data = $node.attr( 'data-sort-value' ); - if ( data !== undefined ) { - return data; + // Use data-sort-value attribute. + // Use data() instead of attr() so that live value changes + // are processed as well (bug 38152). + data = $node.data( 'sortValue' ); + + if ( data !== null && data !== undefined ) { + // Cast any numbers or other stuff to a string, methods + // like charAt, toLowerCase and split are expected. + return String( data ); } else { return $node.text(); } @@ -611,9 +617,16 @@ explodeRowspans( $table ); // try to auto detect column type, and store in tables config table.config.parsers = buildParserCache( table, $headers ); - // build the cache for the tbody cells - cache = buildCache( table ); } + + // Build the cache for the tbody cells + // to share between calculations for this sort action. + // Re-calculated each time a sort action is performed due to possiblity + // that sort values change. Shouldn't be too expensive, but if it becomes + // too slow an event based system should be implemented somehow where + // cells get event .change() and bubbles up to the here + cache = buildCache( table ); + var totalRows = ( $table[0].tBodies[0] && $table[0].tBodies[0].rows.length ) || 0; if ( !table.sortDisabled && totalRows > 0 ) { diff --git a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js index 3a02695c6d..df0144c46e 100644 --- a/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js +++ b/tests/qunit/suites/resources/jquery/jquery.tablesorter.test.js @@ -408,7 +408,8 @@ test( 'bug 32047 - caption must be before thead', function() { test( 'data-sort-value attribute, when available, should override sorting position', function() { var $table, data; - // Simple example, one without data-sort-value which should be sorted at it's text. + // Example 1: All cells except one cell without data-sort-value, + // which should be sorted at it's text content value. $table = $( '
' + '' + @@ -424,30 +425,33 @@ test( 'data-sort-value attribute, when available, should override sorting positi data = []; $table.find( 'tbody > tr' ).each( function( i, tr ) { $( tr ).find( 'td' ).each( function( i, td ) { - data.push( { data: $( td ).data( 'sort-value' ), text: $( td ).text() } ); + data.push( { + data: $( td ).data( 'sortValue' ), + text: $( td ).text() + } ); }); }); deepEqual( data, [ { - "data": "Apple", - "text": "Bird" + data: 'Apple', + text: 'Bird' }, { - "data": "Bananna", - "text": "Ferret" + data: 'Bananna', + text: 'Ferret' }, { - "data": undefined, - "text": "Cheetah" + data: undefined, + text: 'Cheetah' }, { - "data": "Cherry", - "text": "Dolphin" + data: 'Cherry', + text: 'Dolphin' }, { - "data": "Drupe", - "text": "Elephant" + data: 'Drupe', + text: 'Elephant' } - ] ); + ], 'Order matches expected order (based on data-sort-value attribute values)' ); - // Another example + // Example 2 $table = $( '
Data
' + '' + @@ -463,28 +467,89 @@ test( 'data-sort-value attribute, when available, should override sorting positi data = []; $table.find( 'tbody > tr' ).each( function( i, tr ) { $( tr ).find( 'td' ).each( function( i, td ) { - data.push( { data: $( td ).data( 'sort-value' ), text: $( td ).text() } ); + data.push( { + data: $( td ).data( 'sortValue' ), + text: $( td ).text() + } ); }); }); deepEqual( data, [ { - "data": undefined, - "text": "B" + data: undefined, + text: 'B' }, { - "data": undefined, - "text": "D" + data: undefined, + text: 'D' }, { - "data": "E", - "text": "A" + data: 'E', + text: 'A' }, { - "data": "F", - "text": "C" + data: 'F', + text: 'C' }, { - "data": undefined, - "text": "G" + data: undefined, + text: 'G' } - ] ); + ], 'Order matches expected order (based on data-sort-value attribute values)' ); + + // Example 3: Test that live changes are used from data-sort-value, + // even if they change after the tablesorter is constructed (bug 38152). + $table = $( + '
Data
' + + '' + + '' + + '' + + '' + + '' + + '' + + '
Data
D
A
B
G
C
' + ); + // initialize table sorter and sort once + $table + .tablesorter() + .find( '.headerSort:eq(0)' ).click(); + + // Change the sortValue data properties (bug 38152) + // - change data + $table.find( 'td:contains(A)' ).data( 'sortValue', 3 ); + // - add data + $table.find( 'td:contains(B)' ).data( 'sortValue', 1 ); + // - remove data, bring back attribute: 2 + $table.find( 'td:contains(G)' ).removeData( 'sortValue' ); + + // Now sort again (twice, so it is back at Ascending) + $table.find( '.headerSort:eq(0)' ).click(); + $table.find( '.headerSort:eq(0)' ).click(); + + data = []; + $table.find( 'tbody > tr' ).each( function( i, tr ) { + $( tr ).find( 'td' ).each( function( i, td ) { + data.push( { + data: $( td ).data( 'sortValue' ), + text: $( td ).text() + } ); + }); + }); + + deepEqual( data, [ + { + data: 1, + text: "B" + }, { + data: 2, + text: "G" + }, { + data: 3, + text: "A" + }, { + data: undefined, + text: "C" + }, { + data: undefined, + text: "D" + } + ], 'Order matches expected order, using the current sortValue in $.data()' ); }); @@ -527,8 +592,8 @@ test( 'bug 32888 - Tables inside a tableheader cell', function() { var $table; $table = $( - '' + - '
header'+ + '
' + + '' + '' + @@ -543,12 +608,13 @@ test( 'bug 32888 - Tables inside a tableheader cell', function() { 'Child tables inside a headercell should not interfere with sortable headers (bug 32888)' ); equals( - $('#32888-2').find('th.headerSort').length, + $( '#mw-bug-32888-2' ).find('th.headerSort').length, 0, 'The headers of child tables inside a headercell should not be sortable themselves (bug 32888)' ); }); + var correctDateSorting1 = [ ['01 January 2010'], ['05 February 2010'], -- 2.20.1
header'+ '' + '
12
A